Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardize command related names #873

Merged
merged 5 commits into from
Feb 16, 2024
Merged

Standardize command related names #873

merged 5 commits into from
Feb 16, 2024

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Feb 15, 2024

Before we expose command extension APIs, I think it'd be beneficial to first standardize (or even modernize) the names we use for command related concepts. This includes namespace, base class, and folder names:

  1. Rename lib/irb/extend-command.rb to lib/irb/command.rb
  2. Rename lib/irb/cmd/*.rb to lib/irb/command/*.rb
  3. Rename test/irb/test_cmd.rb to test/irb/test_command.rb
  4. Rename ExtendCommand to Command and alias it to Command
  5. Rename Nop to Base and alias it to Base

@st0012 st0012 force-pushed the rename-command-constants branch from d66af64 to 5cb1821 Compare February 15, 2024 17:10
@st0012
Copy link
Member Author

st0012 commented Feb 15, 2024

I think we can maybe postpone the raise of deprecation warning. Otherwise, gems need to start requiring IRB release with this PR + start referencing to the new constants to avoid the warning. But at the same time, it won't bring immediate benefit to them, nor us, other than having clearer names in their codebase.

Update: I decided not to raise deprecation warning on old constants.

@st0012 st0012 requested a review from a team February 15, 2024 20:01
@st0012 st0012 force-pushed the rename-command-constants branch from 7856c3d to 30ce967 Compare February 16, 2024 14:31
@tompng
Copy link
Member

tompng commented Feb 16, 2024

It looks like some repository is requiring "irb/cmd/nop". example: ruby/tracer, irbtoos search result

How about adding a file irb/cmd/nop.rb that only require irb/command/base?

1. Rename lib/irb/extend-command.rb to lib/irb/command.rb
2. Rename lib/irb/cmd/*.rb to lib/irb/command/*.rb
3. Rename test/irb/test_cmd.rb to test/irb/test_command.rb
4. Rename ExtendCommand to Command
@st0012 st0012 force-pushed the rename-command-constants branch from 30ce967 to 0d514c7 Compare February 16, 2024 16:37
@st0012
Copy link
Member Author

st0012 commented Feb 16, 2024

@tompng That's a good point 👍
I added the file back but it actually doesn't need to require anything. In both tracer and irbtools (and I assume other tools too), they are requiring irb as well, which currently already loads the base command.

@st0012 st0012 merged commit 462c128 into master Feb 16, 2024
57 checks passed
@st0012 st0012 deleted the rename-command-constants branch February 16, 2024 16:47
matzbot pushed a commit to ruby/ruby that referenced this pull request Feb 16, 2024
(ruby/irb#873)

* Replace ExtendCommand with Command and standardize command related names

1. Rename lib/irb/extend-command.rb to lib/irb/command.rb
2. Rename lib/irb/cmd/*.rb to lib/irb/command/*.rb
3. Rename test/irb/test_cmd.rb to test/irb/test_command.rb
4. Rename ExtendCommand to Command

* Alias ExtendCommand to Command and deprecate it

* Rename Command::Nop to Command::Base

* Not deprecate old constants just yet

* Add lib/irb/cmd/nop.rb back

ruby/irb@462c1284af
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants